Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix RichTextBox mouse flickering #13

Closed
wants to merge 2 commits into from
Closed

Fix RichTextBox mouse flickering #13

wants to merge 2 commits into from

Conversation

Xanfre
Copy link
Contributor

@Xanfre Xanfre commented May 5, 2019

RichTextBox uses WM_SETCURSOR when it is focused to update the mouse cursor to that which is specified in the control's properties, which causes a noticeable flicker when moving the mouse over particular objects in some readme's. We can intercept this to force the mouse cursor to a particular shape while over text, eliminating this issue.

Perhaps now RichTextBox will finally behave itself.

-Intercept WM_SETCURSOR to nullify mouse pointer flickering on some objects in a readme.
@FenPhoenix
Copy link
Owner

Can you give me one or more examples of readmes which cause mouse flicker? I've never seen this happening on my end. Also, this patch prevents the cursor from changing to a hand when mousing over links for me (the code looks like it's allowing for that, but for some reason it doesn't work, at least for me)

@Xanfre
Copy link
Contributor Author

Xanfre commented May 6, 2019

It happens quite often in my experience. The most noticeable offenders are readmes with centered images, for example, Lord Alan's Factory or An Enigmatic Treasure. Moving the mouse to the left of these images will exhibit flickering. Some research suggests that this may only occur on Windows 10, which may be why you have not experienced it.

Additionally, I apologize for neglecting to test the hyperlinks before pushing the commit. It is still possible to click and follow links properly, but the hand cursor indeed does not display. Provided we can check whether the mouse is over a link, it should be very easy to display the proper cursor, though I remain uncertain of an efficient way to do this.

@FenPhoenix
Copy link
Owner

Okay, I see it happening now. On my Win7 box I find it very subtle and I have to really move the mouse around quite a lot for it to be noticeable. Trying it on my Win10 machine just now, it's actually even more difficult to make it happen. I'm really just circling the mouse around and around to the left of the title pic in Enigmatic and only once every couple dozen moves or so is a very brief flash of another cursor happening, so fast I can barely see it. I guess there's some difference in our machines that's causing it to be noticeable for you, but I dunno what it would be. Anyway, I'll do some experimentation and see if I can make the fix work with links. But I would consider this pretty minor personally, as from what I can see you'd have to just be moving your mouse back and forth over and over and watching the screen very closely, and that just doesn't seem like a sensical thing to do?

But if it's as noticeable as you say then I think I do know what you mean - I vaguely remember that at times in the past I've sometimes had a violently flickering mouse cursor (though I can't recall any particular apps off the top of my head) where it would go back and forth like every frame between a pointer and something else. If that's what's happening for you, then it should definitely be fixed. It's just not what it looks like for me, so I dunno.

@FenPhoenix
Copy link
Owner

After extensive testing and research, I've determined what the root cause is. When you mouseover to the left of any line whose content is not left-justified, the mouse cursor wants to switch to the "arrow cursor but pointing right instead of left" cursor. The one that's meant to signify you can select an entire line by clicking once, you know the one? So the cursor switches to that but then is almost immediately overridden by the default IBeam cursor, causing sometimes-visible flicker. I was unable to prevent the flicker while still keeping link-mouseovers working, even after trying the detect-character-index-under-cursor-and-parse-the-string method (index detection itself is unreliable) and trying to listen for EM_LINK messages (almost works, but due to a dumb quirk of the way you have to receive that message, listening for it breaks the cursor-force so you just end up with the flicker anyway).
However, I was able to come up with something that, depending on your viewpoint, is either a correct solution or not a solution at all: I can stop the IBeam cursor from being set when the pointing-right cursor wants to be set. So what it looks like is that any time there's a line with space to the left of it, the cursor will change to a pointing-right cursor. Not sure if that's more or less annoying, but it's an option.

@Xanfre
Copy link
Contributor Author

Xanfre commented May 6, 2019

I would consider the problem the flicker itself rather than the cursor changing, as depending on where it is in relation to the text, it is supposed to change. On this basis, I would find that solution acceptable.

However, this particular behavior is not the only instance of cursor flicker in the readme box. Upon using the autoscrolling function by pressing the mouse scroll button, the cursors, PanNorth and PanSouth, will also flicker between what they are supposed to be and the IBeam. Therefore, preventing the IBeam cursor from appearing when any other cursor wants to be set would be preferable.

@FenPhoenix
Copy link
Owner

And, hey, whaddya know, my fix doesn't prevent the autoscroll from flickering. I'm also having some trouble with the actual scrolled position and the scrollbar position getting desynced when I click around inside the textbox. Honestly... RichTextBox is a janky mess and I'm not keen on trying to fix the last few inches of it. It'd be better to spend my time looking into alternatives and/or dealing with other stuff. I apologize, and thanks for your contributions, but I think I'm going to have to say "won't fix" for now and focus on other priorities.

@Xanfre
Copy link
Contributor Author

Xanfre commented May 6, 2019

Very well. These are certainly minor issues in the grand scheme of things, and problems relating to utility absolutely should take priority. Thank you for taking this into consideration.

Even so, perhaps I will continue to look into this. It is true that RichTextBox is somewhat of a mess, but fixing what makes it so would be rather nice. I would like to hear more about this scroll desynchronization issue, however, as I have not experienced it. Quite frankly, this also depends on whether you would even want the hand cursor to appear over a link. Links still work properly when clicked, and some may argue that the cursor should always be an IBeam in a textbox.

@FenPhoenix
Copy link
Owner

FenPhoenix commented May 7, 2019

I would say a link should definitely have a hand cursor over it no matter what. If not, I'm almost certain I'm going to hear about it, either people mistakenly thinking they aren't clickable and telling me there's a regression where they're not clickable anymore, or telling me there's a bug where there should be a hand cursor. You and I might be fine with an IBeam, but I'm trying to strike a balance for "people in general" and try to make it work like most people would expect. Keeping in mind that I'm a developer and I have to be careful what I accept as "fine" because it's probable that non-developers will have a whole different idea of what makes sense (witness my belated "Show junk" clarification), so I'm just trying my best to make the UX reasonable and being cautious.

To reproduce the desyncing:

-Comment out my OnVScroll and OnEnter code (it adds more problems, so I'll get rid of it, but the below described problem still happens even without any of my modifications)

-Choose The Rebellion of the Builder 2, TROTB2.rtf. Make sure you haven't selected the RichTextBox (an easy way is just to select another mission and then select TROTB2 again). Now click to the left of the title image. Now scroll down a bit, either with the mousewheel or by dragging the scroll bar down. Now click to the left of the line "THIS FM CAMPAIGN REQUIRES THIEF 2 TO BE PATCHED TO AT LEAST V.1.24". The readme insta-scrolls back to the top, but the actual scroll bar still shows it being scrolled down some. Now scroll down with the mousewheel and the scroll position will now pop back to where it should be given the position of the scroll bar plus the extra amount that you just scrolled, meaning it will appear to jump down in too big a chunk. You can keep doing this multiple times as long as you can see enough of the title pic to click to its left.

Again, to me, this is actually worth the tradeoff because your scrolling fix makes it work way better and I'm not often clicking around in readmes anyway, but I'm certain I'm going to hear about it if I release it. Also, I guess we can keep the right-arrow cursor fix in because it seems to work fine and fixing one case of flicker is better than none.

Edit: The reason I'm a little grumpy about this is that when I say RichTextBox is a mess, what I really mean is that the underlying Windows Rich Edit control is a mess, meaning it's a mess on the internal Windows side, not on the open-source .NET side. This particular control is not like others; it doesn't respond to several things that any other control would respond to, like the DoubleBuffered property (it's ignored, and it flickers when it's resized no matter what). In fact, the RichTextBox control is so non-compliant to expected standards that it actually ignores app-wide mouse blocking on its scroll bar. Straight up, every other control ignores input on every part of it when you tell set the mouse blocker to true, but RichTextBox happily defies it right to its face and picks up your mouse input and lets you merrily scroll all you want. Not that that in particular is a problem really, but it illustrates how futile it is to expect to be able to do anything standard to it and be able to assume it will work. If it was just that the .NET wrapper around it was sub-optimal, as is the case with many other controls, then I'd actually be happy to just subclass it and go crazy, but this just makes me exhausted. So, I guess what I'm saying is, I wish you godspeed if you go at it, try not to end up in the loony bin ;P

@Xanfre
Copy link
Contributor Author

Xanfre commented May 7, 2019

I am painfully aware of the woes of the Rich Edit control and I can certainly understand your attitude towards it. Perhaps it is not worth it to put forth efforts to fix its numerous problems, but fixing the more outlying ones does not go without reward; the awful default Rich Edit scrolling, for example. After this cursor mess is sorted, I do not see any reason to keep fiddling with it. If only it was feasible to use this fix while maintaining the hyperlink behavior. Though if you have fixed the flickering right arrow as you have said, that will likely fix most occurrences of the issue.

Relatedly, I seem to be unable to replicate the desynchronization behavior, which makes it difficult for me to isolate the issue. Even clicking around several readmes like a madman yields no abnormal behavior.

@FenPhoenix
Copy link
Owner

I tried it on my Win10 machine and I can't replicate it there either. Should have done that in the first place. In that case I don't care and I'll call it good. I was recently in the process of setting up a Win10 dev environment, looks like I should finish the process and move it on over to avoid diversions like this. Sorry for the trouble!

@Xanfre
Copy link
Contributor Author

Xanfre commented May 7, 2019

Oh, no worries at all, though I do find it curious that this behavior differs between Windows 7 and Windows 10.

As for the cursor flickering, your latest commit seems to fix all instances I observed other than the middle-mouse autoscroll, so I think I will close this request. Perhaps that particular event is triggered by a different notification and can be easily fixed.

@Xanfre Xanfre closed this May 7, 2019
@FenPhoenix
Copy link
Owner

FenPhoenix commented May 7, 2019

According to the official Rich Edit documentation, the latest version is 4.1 and maps to msftedit.dll (and this is the one that .NET is loading, confirmed by looking at the reference source). It makes it sound like there's only one msftedit.dll and it's version 4.1 and that's that, but doing a search on both my Win7 and Win10 machines, I see four different-sized msftedit.dll files:

Win7:
c:\windows\System32\msftedit.dll - 781KB
c:\windows\SysWOW64\msftedit.dll - 579KB

Win10:
c:\windows\System32\msftedit.dll - 3252KB
c:\windows\SysWOW64\msftedit.dll - 2706KB

AL would only be using the x86 version (SysWOW64 folder), but still, that's quite a substantial difference in file size for "the same version" between OSes. No wonder I've been finding differences. Hm.

@Xanfre
Copy link
Contributor Author

Xanfre commented May 7, 2019

Ah! That would indeed account for the inconsistencies we have experienced here. In fact, brief research shows that several versions of Windows ship with separate versions of the Rich Edit Control, with Windows 10 actually shipping with version 8.5.

Here is some quick data I was able to compile.
Version 1.0:
Class: "RICHEDIT"; Library: Riched32.dll; Shipped with Windows 95
Version 2.0:
Class: "RichEdit20W"; Library: Riched20.dll; Shipped with Windows 98
Version 3.0:
Class: "RichEdit20W"; Library: Riched20.dll; Shipped with Windows 2000
Version 3.1:
Class: "RichEdit20W"; Library: Riched20.dll; Shipped with Windows Server 2003
Version 4.1:
Class: "RICHEDIT50"; Library: Msftedit.dll; Shipped with Windows XP SP1
Version 7.5:
Class: "RICHEDIT50"; Library: Msftedit.dll; Shipped with Windows 8
Version 8.5:
Class: "RICHEDIT50"; Library: Msftedit.dll; Shipped with Windows 10

It looks like every version newer than 4.1 loads the same DLL, with Windows versions succeeding XP each shipping with an updated library.

@FenPhoenix
Copy link
Owner

Thanks for the research. I'll keep this in mind in case new versions are released in the future. Much to my horror, I see that Windows 8 has version 7.5 which I can't even test. Yikes. I'll just have to hope it works okay for anyone on Win8! At least Win10's v8.5 seems to be the best so far, ie., it has issues, but its issues are workaroundable more so than Win7's at least.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants